Skip to content

fix: local gateway connections recover after listener or token drift - #1053

Draft
bkudiess wants to merge 2 commits into
openclaw:mainfrom
bkudiess:bkudiess-connection-analysis
Draft

fix: local gateway connections recover after listener or token drift#1053
bkudiess wants to merge 2 commits into
openclaw:mainfrom
bkudiess:bkudiess-connection-analysis

Conversation

@bkudiess

@bkudiess bkudiess commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

What Problem This Solves

Fixes an issue where users connected to a setup-managed local WSL gateway could
be left in misleading re-pair/token states or unstable reconnect loops when a
stale native gateway owned the local port, the WSL gateway was wedged, or a
role-specific device token drifted.

Why This Change Was Made

This makes GatewayConnectionManager the canonical operator/node lifecycle and
user-intent owner, adds a shared local-listener provenance service, and adds a
bounded managed-WSL repair monitor/coordinator. Strong shared/bootstrap
credentials are sent only after the expected local gateway owner is proven;
unknown listeners are never killed, and explicit Disconnect/Stop intent is
never overridden by automatic repair.

The old owners were scattered UI/client reconnect paths and best-effort port
diagnostics. The new owners are the connection manager for lifecycle and
credential recovery, ManagedLocalGatewayPortProvenanceService for endpoint
identity/remediation, and the monitor/coordinator for bounded automatic repair.

User Impact

Setup-managed local gateways now recover automatically from stale listener
collisions, WSL service/VM failures, and role-specific device-token mismatch.
When ownership cannot be proven, the UI reports a local port conflict instead
of incorrectly asking the user to re-pair.

Evidence

  • Repeated native-vs-WSL fault injection removed only the proven obsolete
    OpenClaw native listener, left the task disabled, restored signed
    wslrelay.exe ownership in about 50 seconds, returned the WSL systemd gateway
    to active, and refreshed LastConnected.
  • Final fault-injection trial removed native PID 42840 and restored the expected
    relay without terminating an unknown process.
  • Current host state after rebase: 127.0.0.1:18789 is owned by
    C:\Program Files\WSL\wslrelay.exe (PID 38548);
    OpenClawGateway is active/running (MainPID 483).
  • Feature head 9d418433 launched successfully in isolated mode and remained
    responsive as PID 41088.
  • Current PR head c2e13727 completed the real fresh-distro setup/connect,
    revocation-recovery, and network-recovery E2E shards. This also proved signed
    wslrelay verification under a PowerShell 7 parent and dual-stack Windows
    relay projection onto the expected systemd-owned WSL socket.
  • Final adversarial sign-off on the feature snapshot: GPT-5.6 Sol max 96/100,
    Claude Opus 4.8 95/100, Gemini 3.1 Pro 100/100; no material findings.
  • Post-rebase rubber-duck review of the four upstream-overlap surfaces found no
    material blockers and confirmed model-catalog, settings, exec-approval, and
    connection-recovery behavior remain wired.

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs or instructions
  • Tests or validation
  • Security hardening
  • Chore or infrastructure

Scope

  • Tray or WinUI UX
  • Windows node capability
  • Local MCP or winnode
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Validation

  • .\build.ps1
    • All five projects built successfully.
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore
    • 3,241 passed, 31 skipped, 0 failed.
  • dotnet test .\tests\OpenClaw.Connection.Tests\OpenClaw.Connection.Tests.csproj --no-restore
    • 496 passed, 0 failed.
  • dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore
    • 698 passed, 0 failed.
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore
    • 2,000 passed, 0 failed.
  • Unit/closeout total: 6,435 passed, 31 skipped, 0 failed.
  • CI E2E shards:
    • Setup/connect and MXC: 18 passed, 0 failed.
    • Network recovery: 2 passed, 0 failed.
    • Revocation recovery: 1 passed, 0 failed.

Known telemetry listener tests flaked transiently during exploratory runs.
Their isolated reruns passed, and the complete required closeout rerun passed.

Real Behavior Proof

  • Environment tested: Windows 11 host, WSL2 OpenClawGateway, setup-managed
    loopback gateway on port 18789.
  • PR head or commit tested: c2e13727.
  • Exact steps or command run: launched
    .\run-app-local.ps1 -NoBuild -AllowNonMain -Isolated; inspected the
    address-specific Windows listener and WSL systemd service; repeated controlled
    native-listener collision/recovery trials before the mechanical rebase; ran
    all three CI setup E2E filters on the current head.
  • Evidence after fix: isolated PR-head app remained responsive; expected signed
    relay owns the loopback listener; WSL service is active/running; controlled
    collisions self-repaired without sending strong credentials to or killing an
    unproven owner; current-head fresh-distro setup, revocation, network recovery,
    and keepalive idempotence all passed.
  • Observed result: managed-local connection health recovers, while explicit
    user intent and unknown-listener safety remain authoritative.
  • Screenshot or artifact links verified? (Yes/No/N/A): N/A; copied
    terminal/runtime evidence is included above.
  • Not verified or blocked: current-head non-isolated launch was blocked by the
    global singleton already owned by another worktree (PID 2496), which was not
    terminated. Current-head runtime behavior was instead exercised through the
    real fresh-distro E2E shards; non-isolated collision recovery was exercised
    repeatedly on the identical feature snapshot before the conflict-free rebase.

Security Impact

  • New permissions or capabilities? (Yes/No): No.
  • Secrets or tokens handling changed? (Yes/No): Yes.
  • New or changed network calls? (Yes/No): Yes, local listener/provenance and
    reconnect probes only.
  • Command or tool execution surface changed? (Yes/No): Yes, bounded repair
    can restart the expected WSL service/VM and disable/stop only a fully proven
    obsolete OpenClaw native task/process.
  • Data access scope changed? (Yes/No): No.
  • If any answer is Yes, explain the risk and mitigation: shared/bootstrap
    credentials are now gated by complete Windows IPv4/IPv6 listener capture,
    signed relay, expected-distro systemd MainPID target-port ownership, and
    listener recapture. Destructive remediation additionally proves native
    task/profile/process lineage and PID start identity. Unknown owners fail closed.

Compatibility and Migration

  • Backward compatible? (Yes/No): Yes.
  • Config or environment changes? (Yes/No): No.
  • Migration needed? (Yes/No): Yes, automatic only.
  • If yes, list the exact upgrade steps: none. Existing setup-managed Release,
    Dev, direct-override, and legacy Local (<distro>) records are migrated or
    resolved in place.

Review Conversations

  • I replied to or resolved every bot review conversation addressed by this PR.
  • I left unresolved only conversations that still need maintainer judgment.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 24, 2026
@clawsweeper

clawsweeper Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed July 27, 2026, 3:08 AM ET / 07:08 UTC.

ClawSweeper review

What this changes

This PR adds managed-local gateway listener provenance checks, role-specific token recovery, explicit disconnect intent, and bounded WSL gateway self-repair for setup-managed local connections.

Merge readiness

Blocked until stronger real behavior proof is added - 7 items remain

Keep this draft PR open for human review. The legacy migration currently classifies any migrated loopback gateway as setup-managed when a local setup-state file exists, which can apply automatic WSL repair and credential-gating behavior to a manually configured localhost gateway.

Priority: P1
Reviewed head: ff2ad8b03d21fc0aa546c4cdbef036acc163f821
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The PR presents substantial implementation and test work, but a blocking upgrade defect and non-inspectable current-head behavior proof keep it below merge-ready quality.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: The PR body reports real runs but supplies no inspectable redacted current-head terminal output, runtime log, recording, or artifact, and names an earlier tested SHA rather than the reviewed head. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The PR body reports real runs but supplies no inspectable redacted current-head terminal output, runtime log, recording, or artifact, and names an earlier tested SHA rather than the reviewed head. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 4 items Legacy migration scope: The changed migration derives a managed WSL distro for every local gateway URL from ambient setup state, then persists that ownership metadata on the migrated record without verifying that the legacy localhost endpoint belongs to that setup-managed gateway.
Automatic-repair eligibility: The branch extends managed-local classification so records carrying setup-managed metadata become eligible for keepalive and automatic repair. A falsely classified manual localhost record can therefore receive lifecycle behavior intended only for app-owned WSL gateways.
Repository architecture policy: The PR adds new lifecycle and provenance owners while touching App.xaml.cs, but the changed-file list does not include docs/ARCHITECTURE.md even though repository policy requires recording an ownership transfer in that ledger and adding a guard for high-regression closures.
Findings 2 actionable findings [P1] Restrict setup-managed migration to verified records
[P2] Record the ownership transfer in the architecture ledger
Security Needs attention Avoid adopting unverified local endpoints: The migration grants setup-managed ownership from ambient setup state for any local URL. That ownership influences strong-credential authorization and the bounded task, process, and WSL repair path, so it must be tied to the actual endpoint before merge.

How this fits together

Gateway records feed the connection manager, which resolves credentials and connects the operator and Windows node to a gateway. For setup-managed local WSL gateways, the new path also verifies the listener owner and may perform bounded recovery before reconnecting.

flowchart LR
  A[Stored gateway records] --> B[Gateway migration and selection]
  C[Setup state] --> B
  B --> D[Connection lifecycle]
  D --> E[Listener ownership check]
  E --> F[Credential authorization]
  E --> G[Bounded WSL repair]
  F --> H[Operator and node connection]
  G --> H
Loading

Decision needed

Question Recommendation
Should automatic repair be enabled by default only for positively identified setup-managed gateway records, with all legacy or manually configured localhost records remaining manual until explicit ownership is established? Narrow migration to proven setup ownership: Persist setup-managed metadata only when setup state can prove the same endpoint and preserve all other migrated localhost records as manual.

Why: The patch introduces a new default recovery and credential-gating policy for persisted gateway records. Source inspection shows the migration boundary is broader than the stated app-owned WSL scope, so maintainers must confirm the intended upgrade contract before a mechanical fix can be accepted.

Before merge

  • Restrict setup-managed migration to verified records (P1) - MigrateFromSettingsCore assigns a setup-managed distro to every migrated local URL whenever ambient setup state exists. A manual legacy localhost gateway can therefore become eligible for WSL repair and listener-based credential gating after upgrade. Only persist managed ownership when the setup record proves the same endpoint, and add an upgrade regression case for manual localhost records.
  • Record the ownership transfer in the architecture ledger (P2) - This adds new managed-local lifecycle and provenance owners while changing App composition, but the repository policy requires the corresponding docs/ARCHITECTURE.md ownership update and a guard for high-regression closures in the same PR. Without it, the declared canonical ownership can drift back into the existing god-file paths.
  • Resolve security concern: Avoid adopting unverified local endpoints - The migration grants setup-managed ownership from ambient setup state for any local URL. That ownership influences strong-credential authorization and the bounded task, process, and WSL repair path, so it must be tied to the actual endpoint before merge.
  • Resolve merge risk (P1) - A legacy manually configured localhost gateway can be silently reclassified as app-owned WSL infrastructure during migration, changing its reconnect, credential, and repair behavior after upgrade.
  • Resolve merge risk (P1) - The patch adds token-disclosure controls plus bounded process, scheduled-task, and WSL lifecycle remediation; its default scope needs maintainer confirmation before landing.
  • Resolve merge risk (P1) - The claimed runtime validation is not inspectable for the reviewed head, so recovery, explicit-disconnect preservation, and upgrade behavior remain unproven in a real current-head setup.

Findings

  • [P1] Restrict setup-managed migration to verified records — src/OpenClaw.Connection/GatewayRegistry.cs:357-360
  • [P2] Record the ownership transfer in the architecture ledger — src/OpenClaw.Tray.WinUI/App.xaml.cs:780-783
  • [medium] Avoid adopting unverified local endpoints — src/OpenClaw.Connection/GatewayRegistry.cs:357
Agent review details

Security

Needs attention: The listener and token protections are security-sensitive, but broad legacy localhost adoption can apply those controls and destructive remediation to endpoints not proven to be app-owned.

Review metrics

Metric Value Why it matters
Patch breadth 60 files affected, 6,204 added, 141 removed The change crosses persisted gateway records, connection lifecycle, setup, WinUI, browser control, and E2E tests, so upgrade boundaries need focused human review.
New recovery surface 3 new managed-local repair/provenance services The new monitor, coordinator, and listener-provenance layer can restart WSL and gate credentials, making ownership classification a merge-critical contract.

Merge-risk options

Maintainer options:

  1. Constrain legacy-record migration (recommended)
    Require endpoint-specific setup ownership before setting managed-local metadata, then add upgrade tests covering manual localhost, legacy setup-managed, SSH, and explicit-disconnect records.
  2. Approve the upgrade policy explicitly
    Accept that every legacy loopback record may become WSL-managed and document the new repair and credential-gating behavior for existing users.
  3. Pause the automatic-repair portion
    Split the repair monitor and destructive remediation from the listener-safety work if the ownership migration contract cannot be made narrow and testable.

Technical review

Best possible solution:

Restrict setup-managed migration to records that can be positively linked to the setup-owned endpoint, preserve manual localhost records as manual, document the resulting ownership boundary, and attach redacted current-head upgrade and fault-recovery proof before merge.

Do we have a high-confidence way to reproduce the issue?

No, not as an executed current-head runtime reproduction. The source provides a high-confidence path: migrate a legacy manual localhost record while a setup-state file exists, then observe that it receives setup-managed metadata and becomes eligible for managed-local policy.

Is this the best way to solve the issue?

No. Listener ownership checks are a reasonable safety direction, but using ambient setup state to adopt every migrated localhost record is not the narrowest maintainable upgrade path; ownership must be positively linked to the persisted endpoint.

Full review comments:

  • [P1] Restrict setup-managed migration to verified records — src/OpenClaw.Connection/GatewayRegistry.cs:357-360
    MigrateFromSettingsCore assigns a setup-managed distro to every migrated local URL whenever ambient setup state exists. A manual legacy localhost gateway can therefore become eligible for WSL repair and listener-based credential gating after upgrade. Only persist managed ownership when the setup record proves the same endpoint, and add an upgrade regression case for manual localhost records.
    Confidence: 0.96
  • [P2] Record the ownership transfer in the architecture ledger — src/OpenClaw.Tray.WinUI/App.xaml.cs:780-783
    This adds new managed-local lifecycle and provenance owners while changing App composition, but the repository policy requires the corresponding docs/ARCHITECTURE.md ownership update and a guard for high-regression closures in the same PR. Without it, the declared canonical ownership can drift back into the existing god-file paths.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against acf4ab1d2b02.

Labels

Label changes:

  • add P1: The migration can alter working existing localhost gateway setups after upgrade and may block or redirect their connection lifecycle.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • remove P2: Current review triage priority is P1, so this older priority label is no longer current.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P1: The migration can alter working existing localhost gateway setups after upgrade and may block or redirect their connection lifecycle.
  • merge-risk: 🚨 compatibility: Persisted legacy gateway records can gain setup-managed ownership based on ambient setup state rather than an endpoint-specific migration proof.
  • merge-risk: 🚨 auth-provider: The new ownership classification controls whether strong gateway credentials may be used, so false classification can change established authentication behavior.
  • merge-risk: 🚨 security-boundary: The patch combines credential gating with process, task, and WSL remediation, and a broad ownership migration expands the scope of those controls.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body reports real runs but supplies no inspectable redacted current-head terminal output, runtime log, recording, or artifact, and names an earlier tested SHA rather than the reviewed head. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

  • [medium] Avoid adopting unverified local endpoints — src/OpenClaw.Connection/GatewayRegistry.cs:357
    The migration grants setup-managed ownership from ambient setup state for any local URL. That ownership influences strong-credential authorization and the bounded task, process, and WSL repair path, so it must be tied to the actual endpoint before merge.
    Confidence: 0.94

What I checked:

  • Legacy migration scope: The changed migration derives a managed WSL distro for every local gateway URL from ambient setup state, then persists that ownership metadata on the migrated record without verifying that the legacy localhost endpoint belongs to that setup-managed gateway. (src/OpenClaw.Connection/GatewayRegistry.cs:357, ff2ad8b03d21)
  • Automatic-repair eligibility: The branch extends managed-local classification so records carrying setup-managed metadata become eligible for keepalive and automatic repair. A falsely classified manual localhost record can therefore receive lifecycle behavior intended only for app-owned WSL gateways. (src/OpenClaw.Tray.WinUI/Services/WslKeepAlivePolicy.cs:111, ff2ad8b03d21)
  • Repository architecture policy: The PR adds new lifecycle and provenance owners while touching App.xaml.cs, but the changed-file list does not include docs/ARCHITECTURE.md even though repository policy requires recording an ownership transfer in that ledger and adding a guard for high-regression closures. (AGENTS.md, acf4ab1d2b02)
  • Current-head proof status: The PR body describes real environment runs, but it identifies c2e1372 as the tested current head while the reviewed head is ff2ad8b. It provides no inspectable terminal transcript, runtime log, recording, or linked artifact for the reviewed head's migration and repair behavior. (ff2ad8b03d21)

Likely related people:

  • bkudiess: The only concrete ownership signal available is the two commits that introduce the listener-provenance, migration, and repair design on this branch; current-main feature-history ownership could not be established from the supplied read-only review material. (role: recent implementation contributor; confidence: low; commits: 309130377f3c, ff2ad8b03d21; files: src/OpenClaw.Connection/GatewayRegistry.cs, src/OpenClaw.Connection/GatewayConnectionManager.cs, src/OpenClaw.Tray.WinUI/Services/ManagedLocalGatewayRepairCoordinator.cs)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Fix the legacy migration boundary and add explicit upgrade coverage for manual localhost records.
  • Update the architecture ownership ledger and add the required regression guard.
  • Attach redacted current-head fault-recovery and upgrade proof, then update the PR body for re-review.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-07-24T20:35:30.039Z sha 9d41843 :: needs maintainer review before merge. :: none

bkudiess and others added 2 commits July 26, 2026 23:59
Add trusted endpoint provenance, role-aware credential recovery, explicit operator intent, and bounded managed-WSL repair so local gateway failures recover safely without leaking stronger credentials or overriding disconnects.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve WSL shell variables through stdin, isolate the Windows PowerShell module path for Authenticode verification, and account for relay family translation. Update the keepalive E2E contract to recognize an already-proven setup keepalive without spawning a duplicate.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@bkudiess
bkudiess force-pushed the bkudiess-connection-analysis branch from c2e1372 to ff2ad8b Compare July 27, 2026 07:04
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. P1 Urgent regression or broken agent/channel workflow affecting real users now. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. P2 Normal priority bug or improvement with limited blast radius. labels Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant